Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow escaped interpolation-like sequences in variable defaults #13137

Merged
merged 1 commit into from
Mar 29, 2017

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Mar 28, 2017

The variable validator assumes that any AST node it gets from an interpolation walk is an indicator of an interpolation. Unfortunately, back in f223be1 we changed the interpolation walker to emit a LiteralNode
as a way to signal that the result is a literal but not identical to the input due to escapes.

The existence of this issue suggests a bit of a design smell in that the interpolation walker interface at first glance appears to skip over all literals, but it actually emits them in this one situation. In the long run we should perhaps think about whether the abstraction is right here, but this is a shallow, tactical change that fixes #13001.

@apparentlymart apparentlymart force-pushed the b-variable-defaults-escape branch from 7907971 to 3a90252 Compare March 28, 2017 21:45
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes, but LGTM

config/config.go Outdated
fn := func(ast.Node) (interface{}, error) {
interp = true
fn := func(n ast.Node) (interface{}, error) {
if _, ok := n.(*ast.LiteralNode); !ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment above here please explaining what this is looking for. :) Its obvious for us since we know the HIL AST.

@stack72 stack72 added the core label Mar 29, 2017
The variable validator assumes that any AST node it gets from an
interpolation walk is an indicator of an interpolation. Unfortunately,
back in f223be1 we changed the interpolation walker to emit a LiteralNode
as a way to signal that the result is a literal but not identical to the
input due to escapes.

The existence of this issue suggests a bit of a design smell in that the
interpolation walker interface at first glance appears to skip over all
literals, but it actually emits them in this one situation. In the long
run we should perhaps think about whether the abstraction is right here,
but this is a shallow, tactical change that fixes #13001.
@apparentlymart apparentlymart force-pushed the b-variable-defaults-escape branch from 3a90252 to 3c95d83 Compare March 29, 2017 15:35
@apparentlymart apparentlymart merged commit 76dca00 into master Mar 29, 2017
@grubernaut grubernaut deleted the b-variable-defaults-escape branch May 11, 2017 00:32
@ghost
Copy link

ghost commented Apr 12, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use anything that looks like interpolation in variable defaults
3 participants